Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge insteon_plm and insteon_local to insteon component #16102

Merged
merged 51 commits into from Aug 22, 2018
Merged

Merge insteon_plm and insteon_local to insteon component #16102

merged 51 commits into from Aug 22, 2018

Conversation

teharris1
Copy link
Contributor

Description:

The insteon_plm and insteon_local components both provide Insteon services for the two different interfaces (PLM and Hub respectively). This PR merges the two so that only one component will serve both Insteon PLM and Insteon Hub users. The summary of changes is:

  • Rename insteon_plm component to insteon component (including all platforms)
  • Add Hub connectivity to insteon component (includes an upgrade to the underlying module insteonplm)
  • Add log messages for insteon_plm and insteon_local users to change to the new insteon component
  • Remove insteon_local from all platforms
  • Remove insteon_hub from all platforms since this old component was deactivated in release 0.32

Related issue (if applicable): fixes #13178

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#6057

Example entry for configuration.yaml (if applicable):

# PLM configuration variables
insteon:
  port: SERIAL_PORT

or

# Hub configuration variables
insteon:
  host: HOST
  ip_port: IP_PORT
  username: USERNAME
  password: PASSWORD

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@teharris1
Implement X10
f11a23a
@teharris1
Add X10 after add_device_callback
61370a2
@teharris1
Ref device by id not hex and add x10OnOffSwitch name
5c3e296
@teharris1
Merge remote-tracking branch 'remote/dev' into x10
1a80d73
@teharris1
X10 services and add sensor device
0973bac
@teharris1
Correctly reference X10_HOUSECODE_SCHEMA
f4148cf
@teharris1
Log adding of X10 devices
a3b4aef
@teharris1
Add X10 All Units Off, All Lights On and All Lights Off devices
9035a23
@teharris1
Correct ref to X10 states vs devices
938f08d
@teharris1
Add X10 All Units Off, All Lights On and All Lights Off devices
dcd4ff8
@teharris1
Correct X10 config
7887256
@teharris1
Debug x10 device additions
60038d9
@teharris1
Config x10 from bool to housecode char
868cd03
@teharris1
Pass PLM to X10 device create
d67d29d
@teharris1
Remove PLM to call to add_x10_device
7d4451f
@teharris1
Unconfuse x10 config and method names
fff0518
@teharris1
Correct spelling of x10_all_lights_off_housecode
b71306f
@teharris1
Merge remote-tracking branch 'remote/dev' into x10
ab7754a
@teharris1
Bump insteonplm to 0.10.0 to support X10
247914d
@teharris1
Add host to config options
733d803
@teharris1
Add username and password to config for hub connectivity
b26308d
@teharris1
Add username and password to config for hub
1eb9f1e
@teharris1
Convert port to int if host is defined
4f90a15
@teharris1
Merge remote-tracking branch 'remote/dev' into hub
81276ff
@teharris1
Merge remote-tracking branch 'remote/dev' into hub
6645656
@teharris1
Add KeypadLinc
60d7b8f
@teharris1
Merge remote-tracking branch 'remote/dev' into KeypadLinc
644def9
@teharris1
Merge remote-tracking branch 'remote/dev' into hub
4ec2cd7
@teharris1
Merge remote-tracking branch 'remote/dev' into KeypadLinc
e0964cd
@teharris1
Merge branch 'KeypadLinc' into hub
df750cb
@ghost ghost added the in progress label Aug 21, 2018
balloob
balloob reviewed Aug 21, 2018
View reviewed changes
homeassistant/components/insteon_plm.py Outdated
@asyncio.coroutine
def async_setup(hass, config):
"""Setup the insteon_plm component."""
_LOGGER.warning('The insteon_plm comonent has been replaced by '
Copy link
Member

@balloob balloob Aug 21, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make it a persistent notification for more visibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I added that to both insteon_plm and insteon_local.

@balloob
Copy link
Member

balloob commented Aug 21, 2018

This looks great! Up to you if you want to keep with the warning or want to make it a persistent notification. We probably also want a comment that we should remove the old platforms in a couple of releases.

houndci-bot
houndci-bot reviewed Aug 21, 2018
View reviewed changes
homeassistant/components/insteon_plm.py Outdated
'The insteon_plm has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
title='insteon_local Component Deactivated',
notification_id='insteon_plm')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_plm.py Outdated
hass.components.persistent_notification.create(
'The insteon_plm has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
title='insteon_local Component Deactivated',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_plm.py Outdated

hass.components.persistent_notification.create(
'The insteon_plm has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_plm.py Outdated
_LOGGER.warning('Please see https://home-assistant.io/components/insteon')

hass.components.persistent_notification.create(
'The insteon_plm has been replaced by the insteon component.<br />'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_local.py Outdated
'The insteon_local has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
title='insteon_local Component Deactivated',
notification_id='insteon_local')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_local.py Outdated
hass.components.persistent_notification.create(
'The insteon_local has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
title='insteon_local Component Deactivated',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_local.py Outdated

hass.components.persistent_notification.create(
'The insteon_local has been replaced by the insteon component.<br />'
'Please see https://home-assistant.io/components/insteon',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

homeassistant/components/insteon_local.py Outdated
_LOGGER.warning('Please see https://home-assistant.io/components/insteon')

hass.components.persistent_notification.create(
'The insteon_local has been replaced by the insteon component.<br />'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line missing indentation or outdented

houndci-bot
houndci-bot reviewed Aug 21, 2018
View reviewed changes
homeassistant/components/insteon_local.py Outdated

return True
"""Setup the insteon_local component.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line contains whitespace

homeassistant/components/insteon_plm.py Outdated
@asyncio.coroutine
def async_setup(hass, config):
"""Setup the insteon_plm component.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line contains whitespace

@teharris1
Copy link
Contributor Author

I added the note to remove in release 0.90. I figured this should be plenty of time for people who don't upgrade regularly. I think you are on a 2 week release cycle so this would be 13 releases or 26 weeks.

@balloob balloob merged commit a31501d into home-assistant:dev Aug 22, 2018
@ghost ghost removed the in progress label Aug 22, 2018
@balloob
Copy link
Member

balloob commented Aug 22, 2018

Perfect, awesome work.

@Madelinot
Copy link

Madelinot commented Aug 26, 2018 •
edited

I have a question. I have BOTH a PLM and a Hub connected in my setup and they work just fine. Like this:

insteon_plm:
  port: /dev/ttyUSB0

insteon_local:
  host: 192.168.x.x
  username: !secret insteon_username
  password: !secret insteon_password
  timeout: 60
  port: xxxxx

Will I have to choose which one I want to use or can I configure both at the same time using the new configuration?

@teharris1
Copy link
Contributor Author

Not a use case I had ever considered so, yes. Just out of curiosity, why do you do this?

@Madelinot
Copy link

Well, the main reason is that HA did not support leak sensors connected to the Hub. Maybe that's changed now.

The way I have it set up now is that all my light switches at connected to the Hub and the leak sensors are connected to the PLM. They both work fine.

@teharris1
Copy link
Contributor Author

Yes, all of the devices work with the Hub in the new design. But we should not comment in the PR. We can move the disucssion here: https://community.home-assistant.io/t/merging-insteon-plm-and-insteonlocal/60323/9

@teharris1 teharris1 deleted the hub branch August 27, 2018 03:19
@balloob balloob mentioned this pull request Aug 29, 2018
Merged
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
@teharris1
Merge insteon_plm and insteon_local to insteon component (home-assist…
fc085a6 
…ant#16102)

* Implement X10

* Add X10 after add_device_callback

* Ref device by id not hex and add x10OnOffSwitch name

* X10 services and add sensor device

* Correctly reference X10_HOUSECODE_SCHEMA

* Log adding of X10 devices

* Add X10 All Units Off, All Lights On and All Lights Off devices

* Correct ref to X10 states vs devices

* Add X10 All Units Off, All Lights On and All Lights Off devices

* Correct X10 config

* Debug x10 device additions

* Config x10 from bool to housecode char

* Pass PLM to X10 device create

* Remove PLM to call to add_x10_device

* Unconfuse x10 config and method names

* Correct spelling of x10_all_lights_off_housecode

* Bump insteonplm to 0.10.0 to support X10

* Add host to config options

* Add username and password to config for hub connectivity

* Add username and password to config for hub

* Convert port to int if host is defined

* Add KeypadLinc

* Update config schema to require either port or host

* Solidify Hub and PLM configuration to ensure proper settings

* Update hub schema

* Bump insteonplm version

* Fix pylint and flake issues

* Bump insteonplm to 0.12.1

* Merge insteon_plm and insteon_local to insteon

* Rename insteon_plm to insteon

* Bump insteonplm to 0.12.2

* Flake8 cleanup

* Update .coveragerc for insteon_plm, insteon_local and insteon changes

* Add persistent notification

* Fix reference to insteon_plm

* Fix indentation

* Shorten message and fix grammer

* Add comment to remove in release 0.90

* Hound fix
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@balloob balloob balloob approved these changes

Assignees
No one assigned
Labels
breaking-change cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

insteon local does not work with a lot of devices
5 participants
@teharris1 @balloob @Madelinot @houndci-bot @homeassistant